Could we maybe still leave the TinyMCE empty check in place, but still look at the value when rendering instead of creating a new property? Should have done that in the undo PR because the property makes little sense.

I think will still have to either check for emptiness or filter out empty nodes, in which case the question becomes which one is more performant?

Could we maybe still leave the TinyMCE empty check in place, but still look at the value when rendering instead of creating a new property?

I think we'll want both changes eventually, but agree it could have been done in separate steps.

I think will still have to either check for emptiness or filter out empty nodes, in which case the question becomes which one is more performant?

Well, we should want to be consistent in either case.

Looking at the implementation of isEmpty, it does a fair bit more, but also things we should probably want. I'll want to compare/contrast with the isEmptyInlineBoundary and isEmptyNode helpers in RichText, as they may be targeting different things.

@aduth What's left to do here? Do we still want this? Or do we want if done differently?

@iseulde I think we do want this, as this.isEmpty is an unnecessary cached derived value which can / should be calculated within render where it's used. That said, the previous consideration of filtering empty nodes to ensure that value is empty when the content is empty could be considered a prerequisite. We'll also need to update this with consideration of the string format prop.

Rebased to resolve conflicts, and changed base to master since #4956 has been merged.

In f9f2739, I applied child node filtering during a split to both the before and after fragments. This should address an issue where pressing Enter from the beginning of a block with text would leave the first with a value of [ '' ] (and thus, not considered empty).

I've played around a bit and am not able to find any other cases where this could occur.

I went further in 26be694 after noticing we have many different touch points considering emptiness in different ways. With #4955 keeping the value should in sync, when the editor is empty the value should never be anything other than an empty array. If it is, that's a bug, and should be addressed directly. With getContent using TinyMCE's isEmpty to return an empty array though, I expect this should be mostly fine as-is. It's the splitting functions that were shown to be problematic here.

I also suspect many of the functions changed may be simply remnants of the pre-#4955 state of the component where we couldn't rely on an up-to-date value.

If it is, that's a bug, and should be addressed directly.

That sounds good. I'm fine with trying this.

Thanks @iseulde and @youknowriad !